Add crate version and commit hash to ldk-server#229
Conversation
|
👋 Thanks for assigning @tankyleo as a reviewer! |
| let (shutdown_tx, shutdown_rx) = tokio::sync::watch::channel(false); | ||
|
|
||
| info!("Starting up..."); | ||
| info!("Starting ldk-server version {FULL_VERSION}"); |
There was a problem hiding this comment.
Should we also do this when we rotate the logs ?
|
|
||
| fn main() { | ||
| println!("cargo:rerun-if-changed=build.rs"); | ||
| println!("cargo:rerun-if-env-changed=GIT_HASH"); |
There was a problem hiding this comment.
Can we make sure to update GIT_HASH if HEAD changes after the first from-clean build. Here it seems HEAD could change, and we'd ship a binary with a wrong rev ?
Or perhaps cargo install always re-runs build.rs so we might be good here.
There was a problem hiding this comment.
Also trying to understand how this triggers. After build.rs sets GIT_HASH, what other thing could cause GIT_HASH to change, and trigger build.rs again ?
There was a problem hiding this comment.
Made it so it watches the git hash file so if it changes then we recalc
benthecarman
force-pushed
the
commit-hash
branch
from
9de46b4 to
5d6e906
Compare
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
vibed with codex, pushed the suggestions in a commit, let me know what you think @benthecarman
The findings:
_ Findings
1. High: inherited GIT_HASH overrides the real repository hash in both build scripts: ldk-server/build.rs:33, ldk-server-cli/build.rs:33. Your report is correct. If the shell exports GIT_HASH, the script skips cargo:rustc-env, so env!("GIT_HASH") compiles the caller_s stale value. I reproduced:
GIT_HASH=stale-from-env cargo run -q -p ldk-server-cli -- --version -> ldk-server-cli 0.1.0 (stale-from-env), and same for ldk-server.
Suggested fix: always compute from git rev-parse HEAD when git is available, then emit cargo:rustc-env=GIT_HASH=.... Only fall back to the environment when git is unavailable, or use a package-specific override variable.
2. Medium: packed branch refs can also leave the hash stale: ldk-server/build.rs:28, ldk-server-cli/build.rs:28, ldk-server/build.rs:54. If the current branch exists only in .git/packed-refs, watch_if_exists skips .git/refs/heads/<branch>. The next commit creates that loose ref, but HEAD and
packed-refs do not change, so Cargo does not rerun the build script. I reproduced this in a throwaway clone: after an empty commit advanced HEAD from 5d6e906... to 80648b9..., ldk-server-cli --version still printed 5d6e906....
Suggested fix: print cargo:rerun-if-changed for the symbolic ref path even when it does not exist yet. I validated that Cargo then reruns when Git creates the file.
Checks run: cargo fmt --all -- --check, cargo check -p ldk-server, cargo check -p ldk-server-cli, configured cargo clippy --all-features -- -D warnings -A clippy::drop_non_drop, cargo test -p ldk-server-cli, and cargo test -p ldk-server all passed.
8e8e2ca to
079d5f9
Compare
|
@tankyleo thanks I like those changes. Squashed them into my commits and added you as co-author |
benthecarman
force-pushed
the
commit-hash
branch
from
079d5f9 to
cdff07e
Compare
We will now log the version number and commit hash when starting up. Also will give the full commit hash when doing ldk-server --version to better help debugging things. Co-authored-by: Leo Nash <hello@leonash.net>
Co-authored-by: Leo Nash <hello@leonash.net>
benthecarman
force-pushed
the
commit-hash
branch
from
cdff07e to
f92e959
Compare
tankyleo
left a comment
tankyleo
left a comment
There was a problem hiding this comment.
What do we want to do with docker? Had codex produce this commit below, could also read the external GIT_HASH env var
Date: Wed Jun 24 16:38:56 2026 +0000
Let Docker builds read Git metadata
Stop excluding .git from the Docker build context. The version build
scripts can then discover HEAD during image builds, matching local Cargo
builds when the source comes from a normal checkout.
Keep the existing GIT_HASH environment fallback for builds that do not
have Git metadata available. That fallback remains the last resort after
git rev-parse HEAD fails, and Cargo still reruns the build scripts when
the fallback value changes.
AI-Generated-By: OpenAI Codex
diff --git a/.dockerignore b/.dockerignore
index a4ec515..d21fbcc 100644
--- a/.dockerignore
+++ b/.dockerignore
@@ -1,6 +1,5 @@
target/
**/target/
-.git/
.gitignore
.github/
.claude/
The Docker build excludes .git via .dockerignore, so the build scripts cannot derive the commit hash from git and fall back to "unknown". Declare GIT_HASH as a build arg and export it as an env var so the build scripts can embed it when callers pass the hash via --build-arg. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I'd rather not include the whole git dir in the build. Made it so we can pass the env var as an arg. This way we can also do like |
3 participants
We will now log the version number and commit hash when starting up. Also will give the full commit hash when doing ldk-server --version to better help debugging things.
Also does the same to the cli